Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 17, 2025

This PR updates the pull request template to prompt authors about test coverage in a concise way.

Changes Made

Enhanced "How did/will you validate this change?" section:

  • Updated the guidance comment to explicitly state: "Be explicit about test coverage."
  • Modified the "Unit tests" bullet point to clarify it refers to "existing" unit test coverage
  • Added a new "New / updated tests" bullet point: "I have added or updated tests (summarize below)."
  • Updated the "None" option to require a justification

Enhanced "Merge / deployment checklist" section:

  • Added a new checklist item: "Confirm that tests have been added/updated or are not needed."

Motivation

These changes help improve code quality by:

  1. Making test coverage a mandatory consideration for all PRs
  2. Ensuring authors explicitly think about and document their testing strategy
  3. Giving reviewers better visibility into test coverage before approving changes

This is a documentation-only change that doesn't affect any code functionality. The template has been kept concise to avoid being unnecessarily verbose while still achieving the goal of prompting authors about test coverage.

Original prompt

Update the pull request template to explicitly prompt authors to confirm that changes are tested or to justify why tests are not needed. Modify the "How did/will you validate this change?" section to add a "New / updated tests" bullet with guidance, and add a checklist item under "Merge / deployment checklist" requiring confirmation that tests exist or a justification is provided.

Target file to update: .github/pull_request_template.md
Existing file BlobSha: 3b632d8f378a5cd056644d83dd5823b0a7a9f580

Proposed new contents:

<!--
    For GitHub staff: Remember that this is a public repository. Do not link to internal resources.
                      If necessary, link to this PR from an internal issue and include further details there.

    Everyone: Include a summary of the context of this change, what it aims to accomplish, and why you
              chose the approach you did if applicable. Indicate any open questions you want to answer
              during the review process and anything you want reviewers to pay particular attention to.

    See https://github.com/github/codeql-action/blob/main/CONTRIBUTING.md for additional information.
-->

### Risk assessment

For internal use only. Please select the risk level of this change:

- **Low risk:** Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.
- **High risk:** Changes are not fully under feature flags, have limited visibility and/or cannot be tested outside of production.

#### Which use cases does this change impact?

<!-- Delete options that don't apply. -->

- **Advanced setup** - Impacts users who have custom workflows.
- **Default setup** - Impacts users who use default setup.
- **Code Scanning** - Impacts Code Scanning (i.e. `analysis-kinds: code-scanning`).
- **Code Quality** - Impacts Code Quality (i.e. `analysis-kinds: code-quality`).
- **Third-party analyses** - Impacts third-party analyses (i.e. `upload-sarif`).
- **GHES** - Impacts GitHub Enterprise Server.

#### How did/will you validate this change?

<!-- Delete options that don't apply. Be explicit about test coverage. If no new tests are added, explain why they are not needed. -->

- **Test repository** - This change will be tested on a test repository before merging.
- **Unit tests** - I am depending on existing unit test coverage (i.e. tests in `.test.ts` files).
- **End-to-end tests** - I am depending on PR checks (i.e. tests in `pr-checks`).
- **New / updated tests** - I have added or updated unit / end-to-end tests to cover new or changed functionality (summarize below) OR I am providing a justification below for why test updates are not required.
- **Other** - Please provide details.
- **None** - I am not validating these changes (provide justification below).

<!-- If you selected "New / updated tests" or omitted adding tests, briefly summarize test additions or rationale: -->

#### If something goes wrong after this change is released, what are the mitigation and rollback strategies?

<!-- Delete strategies that don't apply. -->

- **Feature flags** - All new or changed code paths can be fully disabled with corresponding feature flags.
- **Rollback** - Change can only be disabled by rolling back the release or releasing a new version with a fix.
- **Other** - Please provide details.

#### How will you know if something goes wrong after this change is released?

<!-- Delete options that don't apply. -->

- **Telemetry** - I rely on existing telemetry or have made changes to the telemetry.
    - **Dashboards** - I will watch relevant dashboards for issues after the release. Consider whether this requires this change to be released at a particular time rather than as part of a regular release.
    - **Alerts** - New or existing monitors will trip if something goes wrong with this change.
- **Other** - Please provide details.

### Merge / deployment checklist

- Confirm this change is backwards compatible with existing workflows.
- Tests (unit and/or end-to-end) have been added or updated as needed and are passing, OR a justification for not adding tests has been provided.
- Consider adding a [changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) entry for this change.
- Confirm the [readme](https://github.com/github/codeql-action/blob/main/README.md) and docs have been updated if necessary.

This pull request was created as a result of the following prompt from Copilot chat.

Update the pull request template to explicitly prompt authors to confirm that changes are tested or to justify why tests are not needed. Modify the "How did/will you validate this change?" section to add a "New / updated tests" bullet with guidance, and add a checklist item under "Merge / deployment checklist" requiring confirmation that tests exist or a justification is provided.

Target file to update: .github/pull_request_template.md
Existing file BlobSha: 3b632d8f378a5cd056644d83dd5823b0a7a9f580

Proposed new contents:

<!--
    For GitHub staff: Remember that this is a public repository. Do not link to internal resources.
                      If necessary, link to this PR from an internal issue and include further details there.

    Everyone: Include a summary of the context of this change, what it aims to accomplish, and why you
              chose the approach you did if applicable. Indicate any open questions you want to answer
              during the review process and anything you want reviewers to pay particular attention to.

    See https://github.com/github/codeql-action/blob/main/CONTRIBUTING.md for additional information.
-->

### Risk assessment

For internal use only. Please select the risk level of this change:

- **Low risk:** Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.
- **High risk:** Changes are not fully under feature flags, have limited visibility and/or cannot be tested outside of production.

#### Which use cases does this change impact?

<!-- Delete options that don't apply. -->

- **Advanced setup** - Impacts users who have custom workflows.
- **Default setup** - Impacts users who use default setup.
- **Code Scanning** - Impacts Code Scanning (i.e. `analysis-kinds: code-scanning`).
- **Code Quality** - Impacts Code Quality (i.e. `analysis-kinds: code-quality`).
- **Third-party analyses** - Impacts third-party analyses (i.e. `upload-sarif`).
- **GHES** - Impacts GitHub Enterprise Server.

#### How did/will you validate this change?

<!-- Delete options that don't apply. Be explicit about test coverage. If no new tests are added, explain why they are not needed. -->

- **Test repository** - This change will be tested on a test repository before merging.
- **Unit tests** - I am depending on existing unit test coverage (i.e. tests in `.test.ts` files).
- **End-to-end tests** - I am depending on PR checks (i.e. tests in `pr-checks`).
- **New / updated tests** - I have added or updated unit / end-to-end tests to cover new or changed functionality (summarize below) OR I am providing a justification below for why test updates are not required.
- **Other** - Please provide details.
- **None** - I am not validating these changes (provide justification below).

<!-- If you selected "New / updated tests" or omitted adding tests, briefly summarize test additions or rationale: -->

#### If something goes wrong after this change is released, what are the mitigation and rollback strategies?

<!-- Delete strategies that don't apply. -->

- **Feature flags** - All new or changed code paths can be fully disabled with corresponding feature flags.
- **Rollback** - Change can only be disabled by rolling back the release or releasing a new version with a fix.
- **Other** - Please provide details.

#### How will you know if something goes wrong after this change is released?

<!-- Delete options that don't apply. -->

- **Telemetry** - I rely on existing telemetry or have made changes to the telemetry.
    - **Dashboards** - I will watch relevant dashboards for issues after the release. Consider whether this requires this change to be released at a particular time rather than as part of a regular release.
    - **Alerts** - New or existing monitors will trip if something goes wrong with this change.
- **Other** - Please provide details.

### Merge / deployment checklist

- Confirm this change is backwards compatible with existing workflows.
- Tests (unit and/or end-to-end) have been added or updated as needed and are passing, OR a justification for not adding tests has been provided.
- Consider adding a [changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) entry for this change.
- Confirm the [readme](https://github.com/github/codeql-action/blob/main/README.md) and docs have been updated if necessary.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: oscarsj <1410188+oscarsj@users.noreply.github.com>
Copilot AI changed the title [WIP] Update PR template to include test validation requirements Update PR template to explicitly require test coverage or justification Oct 17, 2025
Copilot AI requested a review from oscarsj October 17, 2025 08:10
@oscarsj oscarsj marked this pull request as ready for review October 17, 2025 08:19
@oscarsj oscarsj requested a review from a team as a code owner October 17, 2025 08:19
Copilot AI review requested due to automatic review settings October 17, 2025 08:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the pull request template to make test coverage requirements more explicit and ensure authors either provide tests or justify their absence. The changes aim to improve code quality by making test coverage a mandatory consideration for all PRs.

Key changes:

  • Enhanced validation section with explicit test coverage guidance and new bullet points
  • Added mandatory test coverage checklist item in the merge/deployment section
  • Updated language to clarify existing vs. new test requirements

Copy link
Member

@mbg mbg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unnecessarily verbose and repetitive. Can we make this more concise?

@oscarsj
Copy link
Member

oscarsj commented Oct 17, 2025

@copilot This seems unnecessarily verbose and repetitive. Can we make this more concise?

Co-authored-by: oscarsj <1410188+oscarsj@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 17, 2025

@copilot This seems unnecessarily verbose and repetitive. Can we make this more concise?

Made the template more concise in commit 80f8fb8. Simplified the "New / updated tests" bullet and checklist item to remove verbose OR clauses while still prompting authors about test coverage.

@copilot This seems unnecessarily verbose and repetitive. Can we make this more concise?

Made the template more concise in fa79376. Simplified the "New / updated tests" bullet and checklist item to remove verbose OR clauses while still prompting authors about test coverage.

Copilot AI changed the title Update PR template to explicitly require test coverage or justification Update PR template to prompt for test coverage Oct 17, 2025
Copilot AI requested a review from oscarsj October 17, 2025 09:30
@oscarsj
Copy link
Member

oscarsj commented Oct 17, 2025

Not needed, mentions to testing where already added and good enough

@oscarsj oscarsj closed this Oct 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants